-
Notifications
You must be signed in to change notification settings - Fork 44
Exclude section headers from workspace symbols in R packages #755
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
471e3ce to
ac6e441
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is working well, both in an R package and not!
Just two things here before you merge:
- Can you add a test for this PR to confirm that we still provide section headers for a regular
.qmdfile not in an R package? I set up the infrastructure for compiling and running tests in #757. - Can you update the changelog for the extension?
|
@juliasilge We could add some extension tests for this but I think the LSP would be the more appropriate context for comprehensive tests? |
|
Comprehensive tests would be great, of course, but given the current state of this monorepo, we want to start where we are and have at least some discipline around adding testing with PRs. I think two good options for this PR would be either:
I leave it to your preference! What we want to avoid moving forward is to merge PRs here in this monorepo without any testing at all. |
ac6e441 to
57cdc51
Compare
57cdc51 to
ebe8b5f
Compare
|
Now with tests! The tests are split in two parts corresponding to a regular project and to an R project (i.e. a project with a top-level DESCRIPTION file). The R project tests have to be run in a different workspace but I struggled to find a way of changing the current workspace reliably. To work around this I added a new test configuration with a different workspace folder set to I also added a new launch configuration for debugging tests. This involved adjusting the test build script to generate source maps. I couldn't make breakpoints work though, so for now you can use Note that the debugger launch config does not use the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much!
- This works great in R packages
- This works great in workspaces that are not R packages
- I could run all the tests (new and old) via
yarn test-vscode
|
@vezwork Just a heads up that there are some additions to the testing infrastructure here, just for visibility for you. |
|
@lionel- Before you merge, can you add a line to |
(This reopens #751 after a weird auto-merge, sorry for any confusion.)
Branched from #750.
Addresses posit-dev/positron#8401.
Once activated, the Quarto extension currently scans all md files and exports section headers to workspace symbols. While great generally, this can get in the way of the dev workflow in R packages.
R packages may include a variety of md files:
This causes Quarto to flood workspace symbols with headers that are mostly irrelevant for package development:
Screen.Recording.2025-07-03.at.15.25.47.mov
I think by default (without any user configuration), headers shouldn't be exported as workspace symbols in R packages. But we need to preserve the existing behaviour for other projects.
To fix this, this PR:
Introduces a new setting
quarto.symbols.exportToWorkspace, with values"default","all", or"none". We need an enum with a default case instead of a boolean to provide the default behaviour that depends on the project type. The other two values allow hardcoding the behaviour in a project.When set to
"default", the LSP detects if the project is an R package.While the LSP would ideally be agnostic regarding the project type, having a customisation mechanism for extensions to set the default behaviour for a project would be complicated and require conflict solving if two extensions try to handle the same project.
We already have a tool to detect R packages in the extension module (thanks to @juliasilge to have let me know!). It's a bit complicated because we need to parse the project's DESCRIPTION file if present, so I think we need to use the same implementation in both the LSP and extension. I pulled it up one level in
apps/utils. To keep things light I didn't make it a full node module.The tool in
apps/utilstake a folder path. Then two wrappers inapps/lspandapps/vscodeare in charge of unpacking the relevant workspace folder (via different APIs specific to the LSP and extension) and calling the util.QA Notes
Create an R package with markdown files and make sure the Quarto extension is activated (it activates when navigating to an R file). Look for workspace symbols (cmd T, or
#prefix in command palette). The markdown headers shouldn't be included.In a project that's not an R package, they should be included.
In the R package you can opt into including the headers via
quarto.symbols.exportToWorkspace. In the regular project you can disable them. The setting should immediately apply (at the next request).